Skip to content

Close the dormant holes from the post-merge review - #112

Merged
nedcut merged 2 commits into
mainfrom
harden/smoke-review-followups
Aug 5, 2026
Merged

Close the dormant holes from the post-merge review#112
nedcut merged 2 commits into
mainfrom
harden/smoke-review-followups

Conversation

@nedcut

@nedcut nedcut commented Aug 5, 2026

Copy link
Copy Markdown
Owner

Follow-up to the post-merge review of #110/#111. Three dormant-but-real holes, each now pinned by a test:

Credential guard held only in-process

The block_real_provider_credentials fixture patched publication_runner.load_environment_files — one importer's reference. Subprocess-driven CLI tests re-import everything in the child and re-read .env.local, and scripts/collect_sota_v3_route_evidence.py has its own call site the patch never touched. The guard now works via GM_BENCH_DISABLE_ENV_FILES, checked inside load_environment_files itself, so it covers every importer and crosses the process boundary (children inherit it). New tests pin both the in-process no-op and the subprocess inheritance.

Operator ceiling failed open on an unreadable protocol

_enforce_operator_ceiling returned silently on OSError/JSONDecodeError, so a corrupt or missing protocol file disabled the ceiling — the one fail-open path in a gate that otherwise raises on every malformed input. It now fails closed with an explicit error.

Evidence artifact resolved from the CWD

v3_route_acceptance_issues read evidence_artifact relative to the current directory, which only worked because pytest and the runner both start at the repo root. Relative paths now anchor to the repo root; a new test runs the check from a foreign CWD.

None of the touched files are in _CONTRACT_SOURCES, so the contract fingerprint is unchanged.

Verification (all zero-spend)

  • Full suite: 767 passed (763 prior + 4 new)
  • run_sota_v3_smoke_from_keychain.py --max-spend-usd 150 --dry-run: Keychain record found, execution hash and hiding commitment both verified, all 10 cells render with pinned routes
  • route-preflight --contract sota-v3: all 10 routes pass zero-completion preflight; gpt-5.6-luna and glm-5.2 live rates dropped below their reservations (favorable)

Three fixes, each pinned by a new test:

- Credential guard now holds at the loader's source: the test suite sets
  GM_BENCH_DISABLE_ENV_FILES, which load_environment_files honours
  directly, instead of patching one importer's reference. This covers
  subprocess-driven CLI tests (the child inherits the variable) and the
  route-evidence collector's own load call, neither of which the old
  in-process patch reached.

- _enforce_operator_ceiling fails closed when the protocol file is
  corrupt or missing. An unreadable protocol may hide a committed cap,
  so it now raises like every other malformed input to the gate instead
  of silently disabling the ceiling.

- v3_route_acceptance_issues resolves a relative evidence_artifact path
  against the repo root rather than the CWD, so acceptance no longer
  depends on where the caller happened to run.
Copilot AI lite review requested due to automatic review settings August 5, 2026 23:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@nedcut, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 59 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: dc2d9be1-cf34-47c8-885a-551b21bc3e34

📥 Commits

Reviewing files that changed from the base of the PR and between 099bcff and 1fadf3b.

📒 Files selected for processing (7)
  • gm_bench/environment.py
  • gm_bench/publication.py
  • scripts/run_publication_matrix.py
  • tests/conftest.py
  • tests/test_environment.py
  • tests/test_publication_runner.py
  • tests/test_sota_v3_route_catalog.py
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch harden/smoke-review-followups

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nedcut
nedcut merged commit ac3711d into main Aug 5, 2026
14 checks passed
@nedcut
nedcut deleted the harden/smoke-review-followups branch August 5, 2026 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants